Skip to content

test(cli): stop the onboarding tests from running the real gcloud - #7881

Closed
adityasingh2400 wants to merge 2 commits into
google:mainfrom
adityasingh2400:fix/cli-onboarding-tests-real-gcloud
Closed

adityasingh2400 wants to merge 2 commits into
google:mainfrom
adityasingh2400:fix/cli-onboarding-tests-real-gcloud

Conversation

@adityasingh2400

@adityasingh2400 adityasingh2400 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
On a developer machine with gcloud installed, two onboarding tests in tests/unittests/cli/utils/test_cli_create.py ran the real gcloud CLI.

  • test_handle_login_with_google_option_3 didn't mock gcp_utils.check_adc. The autouse mock_gcp_metadata_off fixture hides Application Default Credentials, so check_adc() is always False in the suite. Every run therefore called login_adc(), i.e. gcloud auth application-default login, which opens a browser sign-in. On CI, gcloud is missing, so the click.Abort the test expects came from the failed login rather than from choosing "3".
  • test_handle_login_with_google_option_2 didn't mock _onboarding.get_gcp_project_from_gcloud. It read the real default project and, since click.confirm is mocked to True, ran gcloud config unset project on the developer's configuration.

Solution:

  • option_3 mocks check_adc like the other handle_login_with_google tests, so it now reaches and covers the "3. Abandon" choice.
  • option_2 mocks get_gcp_project_from_gcloud (no default project), as option_2_unset_project already does with its own value.
  • An autouse _no_real_gcloud fixture in tests/unittests/conftest.py makes subprocess.run and asyncio.create_subprocess_exec raise an AssertionError for any gcloud or gcloud.cmd command, given by name or by full path. Those are the two ways ADK runs gcloud (_onboarding, gcp_utils, cli_deploy, the Cloud Run deployer, and _dev_deploy). It covers every unit test file, so a future test that forgets a mock fails loudly instead of touching the developer's machine. Tests that patch subprocess.run themselves (e.g. the unset_project and *_from_gcloud_fail tests) still override it.
  • The guard uses its own pytest.MonkeyPatch.context(), not the monkeypatch fixture. Requesting monkeypatch from a conftest autouse fixture sets it up before a test module's own autouse fixtures, which changes their teardown order. In tests/unittests/telemetry/test_agent_engine.py, that made _clear_agent_engine_metrics_cache call cache_clear() on the test's patched function.

Test-only change; no library code is touched.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

With the guard alone, without the two mocks, the guard names exactly the two offenders:

E       AssertionError: Test ran the real gcloud CLI: ['gcloud', 'config', 'get-value', 'project']
E       AssertionError: Test ran the real gcloud CLI: ['gcloud', 'auth', 'application-default', 'login']
FAILED ...::test_handle_login_with_google_option_2
FAILED ...::test_handle_login_with_google_option_3

With the fix, I put a logging fake gcloud first on PATH to catch anything that slips through:

$ pytest tests/unittests/cli
1629 passed, 5 skipped, 1 xfailed
fake gcloud calls: 0

I also ran all of tests/unittests with and without the guard (conftest.py from main). The failures are the same 27 tests both times, and they come from my environment, not this change:

  • my venv has opentelemetry 1.44, below the new <=1.45.2 bound (test_release_dependencies.py);
  • macOS stat lacks -c (integrations/vmaas);
  • the telemetry functional recordings don't match the installed instrumentor (test_functional.py, test_node_functional.py).

test_cli_create.py drops from about 9.6s to 0.3s, since nothing waits on gcloud any more. pre-commit run passes on the changed files.

Manual End-to-End (E2E) Tests:

Before the fix, gcloud's own log (~/.config/gcloud/logs/) from running the suite showed the default project being unset, and a new ADC login on every run:

17.42.27 Running [gcloud.config.get-value] with arguments: [SECTION/PROPERTY: "project"]
17.42.27 Running [gcloud.config.unset] with arguments: [SECTION/PROPERTY: "project"]
17.42.37 Running [gcloud.auth.application-default.login] with arguments: []

After the fix, running the CLI tests adds no gcloud log entries.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

None.

The autouse mock_gcp_metadata_off fixture hides Application Default
Credentials, so test_handle_login_with_google_option_3, which didn't mock
check_adc, always ran the real `gcloud auth application-default login`:
on a machine with gcloud installed every test run opened a browser
sign-in, and on CI it only passed because the missing gcloud happened to
raise click.Abort. test_handle_login_with_google_option_2 read the real
default project and, with click.confirm mocked to True, ran the real
`gcloud config unset project`.

Mock both calls, and add an autouse guard that fails any test in the
module that would run the gcloud CLI.
@adityasingh2400
adityasingh2400 force-pushed the fix/cli-onboarding-tests-real-gcloud branch from 0d3891f to 6d61ce0 Compare November 3, 2026 00:49
copybara-service Bot pushed a commit that referenced this pull request Nov 3, 2026
@adk-bot

adk-bot commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Thank you @adityasingh2400 for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit af458fa.

Closing this PR as the changes are now in the main branch.

@adk-bot adk-bot added the merged [Status] This PR is merged label Nov 3, 2026
@adk-bot adk-bot closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged [Status] This PR is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI unit tests run the real gcloud: an ADC browser login on every run, and they unset the default project

3 participants